-
Notifications
You must be signed in to change notification settings - Fork 516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the rustdoc readme #86
Conversation
I should note that (except for the first sentence) this is a straight copy/paste of what @QuietMisdreavus wrote up. |
cc #2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few notes from when i originally wrote this, and questions about integrating it here. I'm not familiar with any processes y'all may have set up, so i'll just leave these remarks as "if you want to change this, here's more information".
Thanks for porting this! I'm glad that we can get rustdoc into the guide here.
src/rustdoc.md
Outdated
Naturally, there's more than just this, and those descriptions simplify out lots of details, but | ||
that's the high-level overview. | ||
|
||
(Side note: this is a library crate! The `rustdoc` binary is crated using the project in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the readme is no longer right next to the code, it would probably be better to rephrase this line to "Side note: librustdoc
is a library crate!" to avoid confusion.
src/rustdoc.md
Outdated
[under way]: https://github.com/steveklabnik/rustdoc | ||
|
||
Rustdoc is implemented entirely within the crate `librustdoc`. After partially compiling a crate to | ||
get its AST (technically the HIR map) from rustc, librustdoc performs two major steps past that to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
partially compiling a crate to get its AST (technically the HIR map)
Is this a valid thing to say? The complete story of what librustdoc does is that it runs "phase 3" of the compilation process to get a TyCtxt
and query the compiler about type information that it can't get from the source code. But saying that it compiles the crate to its HIR felt like a simpler/smoother thing to say in this context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about saying something along the lines of "it runs the compiler up to the point where we have an internal representation of a crate (HIR) and the ability to query the type system (via TyCtxt
)"?
I'm not sure referring to the TyCtxt
as "the type system" is actually correct, but it helps give you the general idea.
|
||
### From soup to nuts | ||
|
||
(alternate title: ["An unbroken thread that stretches from those first `Cell`s to us"][video]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When i first wrote this, i waffled about whether or not to include this reference. My eventual reasoning was that READMEs weren't as formal a context as proper crate documentation, so it was okay to sneak this pun in. Now that the document's being moved here, is this worth keeping?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the light-hearted title 😁
It's kinda hard to put a simple name to this section, so I'm not sure what I'd replace it with if we were to change it. We're essentially trying to exfiltrate information from the guts of the compiler without breaking things due to lifetimes and the various other restrictions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair, the entire document is full of my usual style of "vague headings that only kinda relate to their section via some extended metaphor" blog-post writing. The specific link to the video is because of the Cell
pun - DocContext
and SharedContext
and Cache
are full of RefCell
s because of how they're passed around. "From soup to nuts" is a variation of the idea that the thing being discussed stretches across the whole documentation process, and won out because it fits the rhythm i established with "From crate to clean" and "From clean to crate". Whether or not to keep the video link is up to the accumulated style of the rest of guide, which is an editorial decision i'm not prepared to make. >_>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally like the titles :)
We can change the later if others want a more formal tone...
"`find_testable_code`" in `html/markdown.rs`, it builds up a collection of tests to run before | ||
handing them off to the libtest test runner. One notable location in `test.rs` is the function | ||
`make_test`, which is where hand-written doctests get transformed into something that can be | ||
executed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a longer writeup about make_test
if y'all feel it's worth linking here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its probably a good idea to mention it. I just skimmed through your article and learner a lot about how rustdoc generates tests. It's really quite smart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, totally unrelated, but that gives me an idea for how to build a simple REPL for rust :P
Before moving on to the next major step, a few important "passes" occur over the documentation. | ||
These do things like combine the separate "attributes" into a single string and strip leading | ||
whitespace to make the document easier on the markdown parser, or drop items that are not public or | ||
deliberately hidden with `#[doc(hidden)]`. These are all implemented in the `passes/` directory, one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a duplicate of the sassy little "I actually liked a little earlier..." paragraph?
Both that paragraph and this one talk about walking the crate and removing/altering things according to the [doc(...)]
attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The key difference between the "I lied a littler earlier" and the "Hot potato" paragraphs is that of timing: The visit_ast.rs
processing happens before crate cleaning, but the passes run after it (but before page rendering).
|
||
### From soup to nuts | ||
|
||
(alternate title: ["An unbroken thread that stretches from those first `Cell`s to us"][video]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the light-hearted title 😁
It's kinda hard to put a simple name to this section, so I'm not sure what I'd replace it with if we were to change it. We're essentially trying to exfiltrate information from the guts of the compiler without breaking things due to lifetimes and the various other restrictions.
src/rustdoc.md
Outdated
|
||
[under way]: https://github.com/steveklabnik/rustdoc | ||
|
||
Rustdoc is implemented entirely within the crate `librustdoc`. After partially compiling a crate to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link to the crate root on GitHub?
|
||
## From crate to clean | ||
|
||
In `core.rs` are two central items: the `DocContext` struct, and the `run_core` function. The latter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be worth adding DocContext
to the code index at the end of the guide.
src/rustdoc.md
Outdated
`#[doc=""]` attributes into a separate field of the Attributes struct, present on anything that gets | ||
hand-written documentation. This makes it easier to collect this documentation later in the process. | ||
|
||
The primary output of this process is a clean::Crate with a tree of Items which describe the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be clean::Crate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I believe it is this one: https://github.com/rust-lang/rust/blob/master/src/librustdoc/clean/mod.rs#L132
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh no. I meant there should be back ticks around the word so it gets styles as code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should get backticks since the rest of the document uses those around other type names.
### Hot potato | ||
|
||
Before moving on to the next major step, a few important "passes" occur over the documentation. | ||
These do things like combine the separate "attributes" into a single string and strip leading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're listing the various rustdoc passes, would it be worth using bullet points? E.g.
- collate and clean doc comments
- remove
[doc(hidden)]
items - ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the current list of passes for rustdoc, as given by a kinda-recent nightly:
$ rustdoc +nightly --passes list
WARNING: the 'passes' flag is considered deprecated
WARNING: please see https://github.com/rust-lang/rust/issues/44136
Available passes for running rustdoc:
strip-hidden - strips all doc(hidden) items from the output
unindent-comments - removes excess indentation on comments in order for markdown to like it
collapse-docs - concatenates all document attributes into one document attribute
strip-private - strips all private items from a crate which cannot be seen externally, implies strip-priv-imports
strip-priv-imports - strips all private import statements (`use`, `extern crate`) from a crate
propagate-doc-cfg - propagates `#[doc(cfg(...))]` to child items
Default passes for rustdoc:
strip-hidden
strip-private
collapse-docs
unindent-comments
propagate-doc-cfg
collapse-docs
is necessary because each line of a doc comment is given as a separate doc attribute, and this will combine them into a single string with line breaks between each attribute.
unindent-comments
is necessary because the convention for writing documentation is to provide a space between the ///
or //!
marker and the text, and stripping that leading space will make the text easier to parse by the Markdown parser. (In my experience it's less necessary now that we have a Commonmark-compliant parser, since it doesn't have a problem with headers that have a space before the ##
that marks the heading.)
strip-priv-imports
is necessary because rustdoc will handle public imports by either inlining the item's documentation to the module or creating a "Reexports" section with the import in it. The pass ensures that all of these imports are actually relevant to documentation.
strip-hidden
and strip-private
also remove items that are not relevant for public documentation.
Passing --document-private-items
disables all the passes and only runs collapse-docs
and unindent-comments
, which means that everything in the crate gets documented, not just the public items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this list (verbatim from your comment) to the chapter, but I think we should avoid documenting some of these hidden cmd line flags since they seem unstable...
src/rustdoc.md
Outdated
whitespace to make the document easier on the markdown parser, or drop items that are not public or | ||
deliberately hidden with `#[doc(hidden)]`. These are all implemented in the `passes/` directory, one | ||
file per pass. By default, all of these passes are run on a crate, but the ones regarding dropping | ||
private/hidden items can be bypassed by passing `--document-private-items` to rustdoc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh, that's cool! I wonder if you can use it from cargo
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly. You'd have to use $RUSTDOCFLAGS
or cargo rustdoc
to add additional arguments to rustdoc.
## From clean to crate | ||
|
||
This is where the "second phase" in rustdoc begins. This phase primarily lives in the `html/` | ||
folder, and it all starts with `run()` in `html/render.rs`. This code is responsible for setting up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you reckon this file would be noteworthy enough to warrant a link to its page on GitHub?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... it would be a bit weird IMHO since there are a lot of other files mentioned, and none of them got special treatment...
@Michael-F-Bryan @QuietMisdreavus I think I addressed all of the comments. Let me know if I missed anything. |
Looks good to me. @QuietMisdreavus, do you think this is good to merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple more comments.
@@ -8,6 +8,7 @@ Item | Kind | Short description | Chapter | | |||
----------------|----------|-----------------------------|--------------------|------------------- | |||
`CodeMap` | struct | The CodeMap maps the AST nodes to their source code | [The parser] | [src/libsyntax/codemap.rs](https://github.com/rust-lang/rust/blob/master/src/libsyntax/codemap.rs) | |||
`CompileState` | struct | State that is passed to a callback at each compiler pass | [The Rustc Driver] | [src/librustc_driver/driver.rs](https://github.com/rust-lang/rust/blob/master/src/librustc_driver/driver.rs) | |||
`DocContext` | struct | A state container used by rustdoc when crawling through a crate to gather its documentation | [Rustdoc] | [src/librustdoc/core.rs](https://github.com/rust-lang/rust/blob/master/src/librustdoc/core.rs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Rustdoc]
doesn't have a link target.
src/rustdoc.md
Outdated
|
||
(Strictly speaking, you can fine-tune the passes run and even add your own, but [we're trying to | ||
deprecate that][44136]. If you need finer-grain control over these passes, please let us know!) | ||
|
||
[44136]: https://github.com/rust-lang/rust/issues/44136 | ||
|
||
Here is current (as of this writing) list of passes: | ||
|
||
- `collapse-docs` is necessary because each line of a doc comment is given as a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The structure of this list a little jarring because it's introduced as "here are the passes" but it's written as "why are they here". Specifically, starting off with "(pass) is necessary..." is awkward because you haven't even described what the pass does yet.
src/rustdoc.md
Outdated
`#[doc=""]` attributes into a separate field of the Attributes struct, present on anything that gets | ||
hand-written documentation. This makes it easier to collect this documentation later in the process. | ||
|
||
The primary output of this process is a clean::Crate with a tree of Items which describe the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should get backticks since the rest of the document uses those around other type names.
@QuietMisdreavus I think I have addressed your comments. I added a bit to each bullet to describe what it does first. Let me know what you think :) |
src/rustdoc.md
Outdated
separate doc attribute, and this will combine them into a single string with | ||
line breaks between each attribute. | ||
- `unindent-comments` is necessary because the convention for writing | ||
- `unindent-comments` removes excess indentation on comments in order for | ||
markdown to like it. This is necessary because the convention for writing | ||
documentation is to provide a space between the `///` or `//!` marker and the | ||
text, and stripping that leading space will make the text easier to parse by | ||
the Markdown parser. (In my experience it's less necessary now that we have a | ||
Commonmark-compliant parser, since it doesn't have a problem with headers | ||
that have a space before the `##` that marks the heading.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this parenthetical is worth keeping? It's definitely not adding as-is, since the rest of the document has so far avoided first-person. Maybe having a historical note that the previous Markdown parser had a problem with spaces before a heading would be useful, but i'm less confident about the remark about it being "less necessary" since i'm not sure that Pulldown is totally okay with everything having an indent on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better?
Looks good to me! Sorry about the back-and-forth there at the end. This has my 👍 now. |
Not at all! Thanks for taking the time 😄 |
…dreavus Move librustdoc readme to rustc guide cc rust-lang/rustc-dev-guide#2 and rust-lang#48478 Don't merge this before rust-lang/rustc-dev-guide#86
cc @QuietMisdreavus @Michael-F-Bryan
After this merges, I hope to remove the README from librustdoc so that we can have everything in one place.